Skip to content

feat(cdk-experimental/ui-patterns): tabs ui pattern #30568

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 15, 2025

Conversation

ok7sai
Copy link
Contributor

@ok7sai ok7sai commented Feb 28, 2025

No description provided.

@ok7sai ok7sai requested a review from a team as a code owner February 28, 2025 18:20
@ok7sai ok7sai requested review from crisbeto and andrewseguin and removed request for a team February 28, 2025 18:20
@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Feb 28, 2025
@ok7sai ok7sai force-pushed the ng-aria-tabs branch 2 times, most recently from f82703c to eed8a03 Compare March 4, 2025 18:04
@wagnermaciel wagnermaciel requested a review from jelbourn March 4, 2025 23:43
'class': 'cdk-tabs',
},
})
export class CdkTabs {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little unsure about having CdkTabs as a necessary wrapper / container vs. the tabpanel having a reference/input for the tablist, implying a 1:1 relationship between tablist and tabpanel.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to include the decision making of choosing a top-level wrapper in the design doc for later. Here's a quick comparison.

Passing references

<ul cdkTablist>
  <li cdkTab #tab1="tab">tab1</li>
  <li cdkTab #tab2="tab">tab2</li>
</ul>
<div cdkTabpanel [for]="tab1"></div>
<div cdkTabpanel [for]="tab2"></div>

Note that the template variable referencing can be reversed. The only dealbreaker I can think of is the dynamic generated tabs or tabpanels via control flow may be impossible. Another downside is the templating effort of creating template references for general use case.

The wrapper solution on the other hand can support both implicit and explicit(yet implemented, but likely via a string identifier other than id) tab-tabpanel binding, which I think it's more intuitive. I also found the same pattern used in most similar libraries, so it can be familiar to developers as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think we should probably have a discussion on the API(s) we expect an end-developer to define for their own tab implementations, and then how to best accommodate that with the directives. FWIW I think we will have a requirement that there may be elements between the tablist and the tabpanel(s).

For content, I suspect these directives will need to end up being oriented around rendering <ng-template> content.

/** The required inputs to tabs. */
export interface TabInputs extends ListNavigationItem, ListSelectionItem, ListFocusItem {
tablist: Signal<TablistPattern>;
tabpanel: Signal<TabpanelPattern>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems odd to me that the tab would need to know about the tabpanel. I was imagining the relationship would be that the tabpanel only knows about the value of the tablist, not necessarily any of the specifics tabs

}

/** A tabpanel associated with a tab. */
export class TabpanelPattern {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably a bigger conversation, but I wonder whether it makes sense to have a tabpanel pattern at all, since it doesn't actually do or render anything.

'class': 'cdk-tabs',
},
})
export class CdkTabs {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think we should probably have a discussion on the API(s) we expect an end-developer to define for their own tab implementations, and then how to best accommodate that with the directives. FWIW I think we will have a requirement that there may be elements between the tablist and the tabpanel(s).

For content, I suspect these directives will need to end up being oriented around rendering <ng-template> content.

@ok7sai ok7sai force-pushed the ng-aria-tabs branch 2 times, most recently from c87bd4c to 91dacdf Compare March 13, 2025 21:30
@ok7sai ok7sai force-pushed the ng-aria-tabs branch 4 times, most recently from 88f775a to d67709a Compare April 15, 2025 06:07
@wagnermaciel wagnermaciel added target: major This PR is targeted for the next major release action: merge The PR is ready for merge by the caretaker labels Apr 15, 2025
@andrewseguin andrewseguin merged commit 9caed2a into angular:main Apr 15, 2025
9 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action: merge The PR is ready for merge by the caretaker detected: feature PR contains a feature commit target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants